Skip to content

Rework the protobuf ecosystem #28321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

suhailskhan
Copy link
Contributor

Description

This is a follow-up to #28098.

Work in progress; see comment below for description of what this PR is intended to do.

Cc @mascguy

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 15.4.1 24E263 arm64
Xcode 16.3 16E140

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • checked your Portfile with port lint?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

[skip notification]

@suhailskhan

This comment was marked as outdated.

@mascguy
Copy link
Member

mascguy commented May 3, 2025

The intent of this PR is as follows:

  • Move protobuf-cpp to protobuf2-cpp and segregate it; update dependent ports to accommodate for the change
  • Segregate protobuf3-cpp
  • Create a new, unsegregated protobuf-cpp port that tracks the latest stable release, replacing the segregated protobuf3-cpp-upstream port
  • Ports that currently depend on protobuf3-cpp most likely should work with the the new protobuf-cpp port, so modify these ports to use it instead of protobuf3-cpp

So far, this PR only implements the first bullet point. The remaining ones are only planned, and have not been done yet. This is only a proposal for one possible approach, I am open to scrapping this and taking a different approach based on feedback.

I definitely support this.

One of our older tickets has plenty of details regarding the challenges for some of our older ports:

https://trac.macports.org/ticket/57117

But as long as we take our time, and work through the details, this would be a nice improvement.

I'm going to try and pull these changes down over the next few days, and review how everything fits together.

And finally, thank you so much for taking the initiative on this!

@suhailskhan

This comment was marked as outdated.

@suhailskhan

This comment was marked as resolved.

@mascguy

This comment was marked as resolved.

@suhailskhan

This comment was marked as resolved.

@mascguy mascguy force-pushed the protobuf-cpp branch 2 times, most recently from 5cc2a3b to 9c33cec Compare May 7, 2025 17:22
@mascguy mascguy force-pushed the protobuf-cpp branch 4 times, most recently from 4fd3cf1 to 3142585 Compare May 7, 2025 18:19
@suhailskhan

This comment was marked as outdated.

@suhailskhan suhailskhan force-pushed the protobuf-cpp branch 2 times, most recently from 50d3434 to ecd852f Compare May 7, 2025 19:29
@suhailskhan

This comment was marked as outdated.

@suhailskhan

This comment was marked as outdated.

@mascguy

This comment was marked as outdated.

@barracuda156
Copy link
Contributor

@suhailskhan @mascguy Maybe follow some consistent naming scheme for protobuf? Current choices appear quite confusing, IMO.

@suhailskhan
Copy link
Contributor Author

Protobuf’s naming and versioning is quite confusing in general (uniquely so), MacPorts aside. Before delving into that, allow me to recap where things currently stand with the core (i.e. C++) Protobuf ports, naming and versioning wise:

  • protobuf-cpp - pinned at v2.x.x
  • protobuf3-cpp - pinned at v3.x.x (v3.21.12, equivalent to v21.12 from the releases page)
  • protobuf3-cpp-upstream - tracks latest release, although out-of-date for a few months (v3.29.2; should have been called v5.29.2 according to the docs; the 3 is just hardcoded in the portfile, and I suppose the in name of the port as well)
  • protobuf5-cpp - pinned at v5.x.x (last 5.x.x release will be 5.29.x, equivalent to v29.x from the releases page, hence it being pinned this way)

The first two among these are installed in ${prefix}, while the rest are in ${prefix}/libexec/${name}.

Now, here is what the PR is doing in its current state:

  • protobuf-cpp -> protobuf2-cpp - move to libexec
  • protobuf3-cpp - no change in name; move to libexec
  • protobuf3-cpp-upstream
  • protobuf - tracks latest release, and uses vX.x versioning instead of vX.x.x; installed in ${prefix}
  • protobuf5-cpp -> protobuf29 - switch to vX.x versioning like protobuf

As for the reasoning and motivation behind the new way of handling the naming and versioning of protobuf, see my earlier comment:

After giving this some thought, I want to suggest we make a change in semantics: call the new protobuf-cpp port protobuf, while keeping the names of the existing protobuf2-cpp and protobuf3-cpp the same. The version number would be 30.2 rather than 6.30.2.

This makes the naming and versioning more consistent with how the developers predominantly refer to it in the documentation. Back when the protobuf-cpp and protobuf3-cpp ports were originally created, protobuf used to be released in separate distributions per language, so it made sense to have “cpp” in the name. After the 22.x release, that had changed. The package containing the C++ bindings is now essentially the protobuf.

So this may make you wonder, why not move protobuf3-cpp to protobuf21, similar to what I did with protobuf5-cpp?

I did this with protobuf5-cpp because this port is a bit over a month old and has no dependents, and I suspect is not yet very well-known, so a name change like this is safe.

On the other hand, protobuf3-cpp has been around for years, and at least some from the MacPorts developers and broader community must be used to this port. Granted, this PR switches practically all of its dependents away from the port anyway, barring a couple or so for which it is not possible to do so. So technically, doing such a rename is not highly consequential in that sense. However, this is legacy software now (as is protobuf2-cpp), and so I thought for that reason alone it isn’t worth the rename. It is a matter of time before this port is marked as obsolete, the way I see things with respect to its usefulness.

I am still open to making protobuf3-cpp consistent with protobuf, but I just have not since I did not see it to be necessary or advisable, for the reasons I have just stated. I also figured this discussion would come up sooner or later as a result of that choice, which is another reason I did it; to know for sure whether or not to leave protobuf3-cpp the way it is.

@barracuda156
Copy link
Contributor

@suhailskhan Thank you for a detailed explanation. My concern was the following: without knowing all this, it may appear that protobuf29 stands for 2.9 (like python27 stands for 2.7), and thus is older than protobuf3, while in fact the reverse is true. It is also confusing that in some cases -cpp is retained, in other dropped, while both are identically representing protobuf C++.

@suhailskhan
Copy link
Contributor Author

suhailskhan commented May 19, 2025

without knowing all this, it may appear that protobuf29 stands for 2.9 (like python27 stands for 2.7), and thus is older than protobuf3, while in fact the reverse is true.

I had this exact concern too, though when considering how people will discover the port, through means such as port search or ports.macports.org, the actual version number would be immediately apparent in both of those cases. Maybe having a hyphen before the “29” would somehow make it clearer? I am not sure that helps, or even aligns with typical naming conventions for ports, but that was something I considered but ultimately did not push.

It is also confusing that in some cases -cpp is retained, in other dropped, while both are identically representing protobuf C++.

This again comes back to those two older ports being legacy. If we go ahead with keeping the names unchanged, we would just need to endure this until their eventual deletion, which would hopefully be very soon. But if we know that for some reason they will need to stick around for relatively long, then perhaps a rename right now is justifiable. I am not sure I can make a strong call one way or another as I don’t have enough insight, but from what limited insight I do have, they would probably not need to stay for too much more time.

Another thing that can be done irrespective of this, and that I had been considering to do, is add a notes block to these two ports to make people aware that they might be looking for protobuf, and update the description to say something along those lines as well. In any case, it is probably good to have a notes block because even if someone knowingly installed protobuf3-cpp, they would probably need to know that they have to add ${prefix}/libexec/protobuf3-cpp/bin to their PATH to use protoc etc.

@suhailskhan
Copy link
Contributor Author

suhailskhan commented May 19, 2025

I am not sure I can make a strong call one way or another as I don’t have enough insight, but from what limited insight I do have, they would probably not need to stay for too much more time.

What I probably do have enough insight to say is that protobuf-cpp (renamed to protobuf2-cpp in this PR) could be deleted as soon as today. It has three dependents, two of which are its respective Java and Python bindings, and the other is an obscure port that has been broken for some time now. The Java bindings have no dependents, and the Python bindings have one dependent: a very out-of-date port that does not consistently build on all macOS versions. It appears that upstream does not even have a Protobuf dependency anymore, so if it were to be updated then py-protobuf (renamed to py-protobuf2 in this PR) would be completely unnecessary.

@barracuda156
Copy link
Contributor

barracuda156 commented May 19, 2025 via email

@suhailskhan suhailskhan mentioned this pull request May 19, 2025
11 tasks
barracuda156 added a commit to macos-powerpc/powerpc-ports that referenced this pull request May 21, 2025
@mascguy
Copy link
Member

mascguy commented May 22, 2025

I haven't forgotten about this, work has simply been super-busy this week.

Worse-case, I'll try to make time this weekend.

@mascguy
Copy link
Member

mascguy commented May 29, 2025

I finally had a chance to uninstall the various protobuf ports, grpc, etc, and reinstall the latest ones. There's still more to test, but so far, so good.

One thing I noticed though: For the current dependents of py-protobuf3, have you tried using py-protobuf instead? That would allow us to eliminate that port, further simplifying things.

Those ports are:

  • py-google-api-core
  • py-googleapis-common-protos
  • py-siphon
  • qgis3
  • qgis3-ltr

If we do have to keep both py-protobuf and py-protobuf3, you'll need to declare a conflict declaration against the peer port. (Since they both install into the same prefix.)

@suhailskhan
Copy link
Contributor Author

suhailskhan commented May 30, 2025

My apologies for the confusion that attempting to deal with the size of these PRs has caused, but these ports have already been updated to use the new py-protobuf in separate PRs, again to avoid the checks from timing out had those updates been part of this PR.

  • py-google-api-core
  • py-googleapis-common-protos
  • py-siphon
  • qgis3
  • qgis3-ltr

@suhailskhan
Copy link
Contributor Author

I agree that we should get rid of py-protobuf3 altogether. I had initially begun by opting to make the least possible changes to it, and had not added a conflict declaration, until we determine whether or not this port should even stay.

@mascguy
Copy link
Member

mascguy commented May 30, 2025

My apologies for the confusion that attempting to deal with the size of these PRs has caused, but these ports have already been updated to use the new py-protobuf in separate PRs, again to avoid the checks from timing out had those updates been part of this PR.

Sounds good, I completely forgot about the separate PRs. (It's been a very long two weeks at work.) I'll pull more of those down and test.

But overall, it looks like we're getting close to being able to merge all of your great work!

The only blocker, is our buildbot infrastructure: Presently, the builders are down, due to storms in Texas. So we'll want to wait until that is back up - and fully caught up - before merging. We don't have an ETA yet, but it could be anywhere from a few days, up to a week.

In the interim, that gives me plenty of time to continue validating locally. And I'll continue to do that over the weekend.

More to follow, but I'm very excited that we're almost there. And thank you again for all of the time you've put into this effort, as it will make a huge improvement to MacPorts as a whole!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants